-
Notifications
You must be signed in to change notification settings - Fork 409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#1449] feat(catalogs): Introudce new module bundled-catalog
for query engine.
#1454
Conversation
...va/com/datastrato/gravitino/trino/connector/catalog/jdbc/mysql/MySQLDataTypeTransformer.java
Outdated
Show resolved
Hide resolved
exclude("**/*.conf") | ||
exclude("**/*.so") | ||
exclude("**/*.sxd") | ||
exclude("**/*.ddl") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you exclude everything and only leave something you wanted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
catalogs/catalog-common/src/main/java/com/datastrato/catalog/common/ClassProvider.java
Outdated
Show resolved
Hide resolved
...g-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveSchemaPropertiesMetadata.java
Outdated
Show resolved
Hide resolved
@jerryshao @diqiu50 |
catalogs/catalog-common/src/main/java/com/datastrato/catalog/common/ClassProvider.java
Outdated
Show resolved
Hide resolved
.../com/datastrato/gravitino/trino/connector/catalog/hive/TestHiveCatalogPropertyConverter.java
Outdated
Show resolved
Hide resolved
@@ -77,12 +77,12 @@ dependencies { | |||
tasks { | |||
val copyDepends by registering(Copy::class) { | |||
from(configurations.runtimeClasspath) | |||
into("build/libs") | |||
into("build/libs_all") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build
and copyDepends
tasks of catalog-hive
use the same output directory 'build/libs', which is needed by task shadowJar
in bundled-catalog
, gradle will get errors and says "task shadowJar
depends on task copyDepends
".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'd better add comment on the file to describe it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added.
.../com/datastrato/gravitino/trino/connector/catalog/hive/TestHiveCatalogPropertyConverter.java
Outdated
Show resolved
Hide resolved
catalog-common
for query engine.bundled-catalog
for query engine.
What changes were proposed in this pull request?
bundled-catalog
to hold common information like property meta of all catalogsbundled-catalog
totrino-connector
dependency and verify everything is OK.Why are the changes needed?
We need a common module to hold the property meta of all catalogs so that the query engine depends on it can use it for query.
Fix: #1449
Does this PR introduce any user-facing change?
N/A
How was this patch tested?
Add some UT in module
Trino-connector